GH-47389: [Python] CSV and JSON options lack a nice repr/str - #47397
Conversation
AlenkaF
left a comment
There was a problem hiding this comment.
Thanks for working on this @thisisnic!
My first thought was whether we could simplify this to avoid hardcoding all the options, but nothing simpler comes to mind — I don't think we can easily loop over attributes in a Cython cdef class.
So overall, no big concerns — just one note about a missing option and a small nit.
AlenkaF
left a comment
There was a problem hiding this comment.
Thanks for the changes! Two more comments from me.
b1177bc to
2b3354b
Compare
|
@pitrou mind having a quick look? |
fdcdefe to
82dc9ad
Compare
Co-authored-by: Alenka Frim <AlenkaF@users.noreply.github.com>
82dc9ad to
552771e
Compare
|
Thanks for the changes @thisisnic! |
|
Thanks @thisisnic ! |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 618d5fb. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them. |
…pache#47397) ### Rationale for this change CSV and JSON options lack a nice repr/str dunder method ### What changes are included in this PR? Add both these methods ### Are these changes tested? Will be once it's ready for review ### Are there any user-facing changes? No * GitHub Issue: apache#47389 Lead-authored-by: Nic Crane <thisisnic@gmail.com> Co-authored-by: AlenkaF <frim.alenka@gmail.com> Co-authored-by: Alenka Frim <AlenkaF@users.noreply.github.com> Signed-off-by: AlenkaF <frim.alenka@gmail.com>
…pache#47397) ### Rationale for this change CSV and JSON options lack a nice repr/str dunder method ### What changes are included in this PR? Add both these methods ### Are these changes tested? Will be once it's ready for review ### Are there any user-facing changes? No * GitHub Issue: apache#47389 Lead-authored-by: Nic Crane <thisisnic@gmail.com> Co-authored-by: AlenkaF <frim.alenka@gmail.com> Co-authored-by: Alenka Frim <AlenkaF@users.noreply.github.com> Signed-off-by: AlenkaF <frim.alenka@gmail.com>
Rationale for this change
CSV and JSON options lack a nice repr/str dunder method
What changes are included in this PR?
Add both these methods
Are these changes tested?
Will be once it's ready for review
Are there any user-facing changes?
No